TabView.ItemChanged Event Fix #8420
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
microsoft-github-policy-service
bot
added
the
needs-triage
karkarl
commented
Apr 20, 2023
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
kmahone
reviewed
Apr 24, 2023
| // TODO 19727004: Re-enable this on versions below RS5 after fixing the bug where mouse click-and-drag doesn't work. | ||
| Log.Warning("This test relies on touch input, the injection of which is only supported in RS5 and up. Test is disabled."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
You can actually remove this, since we no longer run tests lower than RS5.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bkudiess
pushed a commit
that referenced
this pull request
May 1, 2023
* re-enable OnItemsChanged OnDrag * update testPage * add test * undo accidental change * fix test * add test * remove outdated check * fix test * fix test (cherry picked from commit c8d3b4a)
DHowett
pushed a commit
to microsoft/terminal
that referenced
this pull request
May 10, 2023
Reverts #15164, because that's fixed upstream now. Closes #15139. Reverts #15178, but also closes #15121, because that's fixed upstream. see also: * microsoft/microsoft-ui-xaml#8430 * microsoft/microsoft-ui-xaml#8420
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
https://github.com/microsoft/microsoft-ui-xaml/blob/main/dev/TabView/TabView.cpp#L725
From #6632 to fix internal bug 37608918 caused a regression where TabView.ItemChanged event is simply not raised.
Upon investigation, normally,
OnItemsChangedis raised twice when dragged past the overflow scroll buttons:CollectionChanged::ItemRemoved- Dragged item being removed since it's "overflow"CollectionChanged::ItemInserted- Dragged item being dropped back inSince TabView thinks the tab is being removed, it finds the closest tab to give selection: https://github.com/microsoft/microsoft-ui-xaml/blob/main/dev/TabView/TabView.cpp#L764
Fix:
Remove the
m_isDraggingcheck and simply callBringSelectedTabIntoView()OnListViewDragItemsCompletedfor the original bug fix.Removing the
m_isDraggingcheck also prevents a regression where a tab is being dragged to another window, causing no tab to be selected.(This is a regression)
Motivation and Context
Fixes internal bug 44190134
How Has This Been Tested?
Added new test
ItemChangedEventOnDragTestto avoid regression where ItemChanged Event is not called.Screenshots (if appropriate):